-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(vite): close server and exit if stdin ends #1857
Conversation
This PR was merged, but the underlying problem still persists as the following commit reverts what this PR fixed. In the current state vite still doesn't stop when stdin is closed and creates zombie processes when used with phoenix. |
@LostKobrakai For Phoenix/Elixir specifically there is a wrapper script you can use, the Port docs talk about Zombie processes and how to prevent them. As for the PR itself, I would have liked to get the support directly into Vite before the v2 release of course, but the solution might be more complex than a quick PR after all. Sorry for causing the need to roll back the change. The tests passed and I didn't have the reference frame to notice something's not right beyond that. |
@nullpilot I get weird error when trying to use that work around - using exactly the script they talk about (I called it watchers: [
node: [
"start.sh",
"npx",
"vite",
cd: Path.expand("../assets", __DIR__)
]
However, it gives me errors as if it's not even interpreting it with bash, if I remove all comments I still get unexpected token with some valid (and needed bash syntax):
|
For anyone who lands here I fixed it with const { execSync } = require("child_process");
// Exit the process when standard input closes due to:
// https://hexdocs.pm/elixir/1.10.2/Port.html#module-zombie-operating-system-processes
//
process.stdin.on("end", function() {
console.log("standard input end");
process.exit();
});
process.stdin.resume();
execSync("./node_modules/.bin/vite")
...
config :demo, DemoWeb.Endpoint,
http: [port: 4000],
debug_errors: true,
code_reloader: true,
check_origin: false,
watchers: [
node: [
"start.js",
cd: Path.expand("../assets", __DIR__)
]
]
... |
I could not make @jonathanstiansen script work. This one works for me, by killing the vite process when receiving a CTRL-D: const { spawn } = require("child_process");
const child = spawn("./node_modules/.bin/vite", { stdio: "inherit" });
process.stdin.on("end", function () {
console.log("stdin close");
child.kill();
process.exit();
});
process.stdin.resume(); |
Hi, just to add more option for folks here. I use the following, and --- a/assets/package.json
+++ b/assets/package.json
@@ -3,8 +3,8 @@
"scripts": {
+ "build": "vite build",
+ "watch": "vite build --watch --minify false --emptyOutDir false --clearScreen false --mode development"
},
"devDependencies": {
+ "vite": "^2.2.4"
}
}
--- a/config/dev.exs
+++ b/config/dev.exs
@@ -23,7 +23,7 @@ config :fset, FsetWeb.Endpoint,
watchers: [
- node: ["whatever", "ever", cd: Path.expand("../assets", __DIR__)]
+ yarn: ["run", "watch", cd: Path.expand("../assets", __DIR__)]
] For full integration (least work), check this out: https://github.com/50kudos/vite-phx |
This seems to cause problems with the vite web-test-runner plugin: material-svelte/vite-web-test-runner-plugin#6. |
@nullpilot @IanVS I've been noticing a weird amount of memory usage while using Vite in my WSL machine while starting/ stoping phoenix while using Vite. Is this a regression? I looked at this comment over here: #3659 (review) which seems to suggest that this is the case. Is anyone else having problems with this? |
@thiagomajesk I attempted to add |
@thiagomajesk This PR caused some other side effects (sorry about that!) and was partially rolled back - removing the line that was the actual fix for this particular issue. I haven't kept up and don't know what the current situation on this is. The Elixir docs provide an example of a wrapper script that can be used to avoid this issue, as far as I understand. I haven't tried since my setup changed, but that's what I would go with now. |
Hey, @IanVS I'm no expert either, so I don't know how far I can contribute to this one. However, I read a post recently that uses a similar approach. It looks a bit hackish but it seems to work: https://moroz.dev/blog/integrating-vite-js-with-phoenix-1-6. |
@nullpilot I was wondering if a |
@thiagomajesk It would make sense and I think it would be the most reliable solution at this point. When I made this PR I specifically looked for an option that didn't require a new flag, instead of proposing the solution from vue-cli, as it was stated somewhere that it's the preferred solution over extra flags if they can be avoided. Given that the no-flag solution hasn't really worked out, it may be worth sticking with the proven method and adding it as a flag after all. |
Description
When running Vite from a parent process (Elixir Phoenix dev watcher in my case) the process doesn't shut down and keeps running in the background after the parent process is stopped.
Example
Starting and then stopping
mix phx.server
with watchers set up to runnpx vite
leaves two ghost processes still running:Solution
In vue-cli the webpack approach was used, and a flag was added to address this (vuejs/vue-cli#1597)
Here I am instead using the same approach used in rollup (rollup/rollup#3493) to do it without adding an extra flag.
With the change, running the same as above no longer leaves any dangling processes running and everything shuts down as expected.